-
Notifications
You must be signed in to change notification settings - Fork 1
Add a list of Windows apps with known compatibility issues #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
ameshkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this format, check how it's done in Android. We may need structured data in the future.
You should also add the description to README
…t decided on yet.
…ner.exe to routing_exclusions.json
…ting_exclusions.json
| ## What AdGuard applications use these filtering lists? | ||
|
|
||
| Currently, they apply to AdGuard for Android. | ||
| Currently, they apply to AdGuard for Android and AdGuard for Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
для макоси и клишных блокеров не актуально?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Пока нет, так как в них еще не реализован System wide filtering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ну почему, актуально для макоси, про это задача есть
…ing_exclusions.json
…ing_exclusions.json
…ing_exclusions.json
…uardTeam/AdguardForWindows#5721 to routing_exclusions.json
…ing_exclusions.json
…ing_exclusions.json
…ing_exclusions.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds Windows-specific compatibility configuration lists to this repo so AdGuard for Windows can consume them alongside the existing Android lists.
Changes:
- Added Windows app exclusion lists for routing and filtering behavior.
- Added a Windows browser list used to enable HTTPS filtering by default for specific browsers.
- Updated README to document Windows compatibility configuration files and their JSON schema.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| windows/routing_exclusions.json | New list of Windows apps whose traffic should not be routed through AdGuard. |
| windows/filtering_exclusions.json | New list of Windows apps whose traffic filtering should be disabled. |
| windows/browsers.json | New list of browsers + install-detection conditions for enabling HTTPS filtering by default. |
| README.md | Documents Windows config files and the common JSON entry structure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ameshkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the PR comments
| ## What AdGuard applications use these filtering lists? | ||
|
|
||
| Currently, they apply to AdGuard for Android. | ||
| Currently, they apply to AdGuard for Android and AdGuard for Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ну почему, актуально для макоси, про это задача есть
| Each application entry in the JSON files uses the following structure: | ||
|
|
||
| - `name` - Application or browser display name | ||
| - `file_description` - File description (optional, can be empty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest renaming file_description to description. As far as I understand it is supposed to be a compatibility issue description right?
|
|
||
| ### Application model structure | ||
|
|
||
| Each application entry in the JSON files uses the following structure: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not explained which files from above should use this structure
| [ | ||
| { | ||
| "name": "iTunes", | ||
| "file_description": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fill the description where it's possible, explain the reason for having it in exclusions.
|
|
||
| - [`https_filtering_apps.json`](windows/https_filtering_apps.json) - A list of non-browser apps where HTTPS traffic filtering is enabled by default (App Management -> Filter HTTPS traffic). | ||
|
|
||
| ### Application model structure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a good idea to have identical structure for browsers.json and for different exclusions lists.
They have different purpose, and some fields only make sense in browsers and do not make any sense in other.
In browsers.json as far as I understand (@adbuker please correct me if I am wrong)
file_description- used for detecting processes to filter?installed_conditions- only makes sense forbrowsers.json
At the same time, public_issue_url and private_issue_id only make sense in the files for exclusions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file_description - used for detecting processes to filter?
Btw, if this is true, I'd say it would be better to structure a browser record like this:
{
"name": "Google Chrome",
"routing_conditions": {
"file_description": "Google Chrome",
"executable_names": [
"chrome.exe"
],
},
"installed_conditions": [
// here go the installed conditions
]
}In this case we'll only need routing_conditions in the exclusions files.
Add a list of Windows apps with known compatibility issues